Feature: Add `<BUILTIN_MODULES>` Special Word #86

fbartho · 2023-05-16T19:31:57Z

This allows users to specify a gap between built-ins and third-party in a slightly less wonky way, and the plugin's default importOrder & related documentation a bit clearer.
This also allows users to move the built-ins around relative to other imports. (Weakens our previous opinionated decision that built-ins should always be first).

- This allows users to specify a gap between built-ins and third-party in a slightly less wonky way. - This also allows users to move the built-ins around relative to other imports.

IanVS

Minor suggestions, but nothing blocking. Nice improvement!

README.md

IanVS · 2023-05-17T12:02:04Z

src/utils/get-import-nodes-matched-group.ts

+    const result = new RegExp(regExp.replace(TYPES_SPECIAL_WORD, ''));
+    regexCache.set(regExp, result);
+    return result;
+};


Nice little perf improvement here.

I wanted also to parse importOrder and normalize it only once, but that requires a further cleanup because there’s logic that looks at the first entry in importOrder and does something if it’s a space.

I decided to submit this PR as-is and consider if it’s worth changing that at some future date.

IanVS · 2023-05-17T12:04:54Z

src/utils/normalize-import-order-options.ts

+
+    importOrder = importOrder.map((g) =>
+        g === BUILTIN_MODULES_SPECIAL_WORD ? BUILTIN_MODULES_REGEX_STR : g,
+    );


Why do this map here, vs just injecting the regex str on line 25?

We could inject the regex-str up on line 25, but it wouldn’t allow for us to remove the .map — if we want to allow users to have the built-in modules in a different place.

Here’s my thought process our code sort of looks like this:

// top-of-file-comment(s) a-z builtin imports a-z other-third-party a-z relative imports runtime.code()

I like gaps between my groups of imports, so we’d have this awkward leading empty string on the array [“”, ”<THIRD_PARTY_MODULES>”,… if there was no word for builtin modules.

And we’d never even be able to express “put a gap above built-ins, but after top-of-file comments” if we didn’t have that special word. — Unfortunately, we still can’t do that (yet) even if importOrder can express it. It would require further logic that I’m still interested in implementing; I think it’s just a (+-)1 on the shifting of the loc you recently added.

Oh right, I was neglecting to consider that we need to support both having the special word in the config and injecting it. The third-party special word is handled elsewhere.

Co-authored-by: Ian VanSchooten <ian.vanschooten@gmail.com>

RegExps are heavy objects to init, cache them

1616925

fbartho requested a review from IanVS May 16, 2023 19:31

Feature: Add <BUILTIN_MODULES> Special Word

7a4b03e

- This allows users to specify a gap between built-ins and third-party in a slightly less wonky way. - This also allows users to move the built-ins around relative to other imports.

fbartho force-pushed the fb/builtin-modules-word branch from a04e7a5 to 7a4b03e Compare May 16, 2023 19:37

IanVS approved these changes May 17, 2023

View reviewed changes

Apply README tweak suggestions from code review

799fb94

Co-authored-by: Ian VanSchooten <ian.vanschooten@gmail.com>

IanVS merged commit dcb19a0 into next May 17, 2023

IanVS deleted the fb/builtin-modules-word branch May 17, 2023 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Add `<BUILTIN_MODULES>` Special Word #86

Feature: Add `<BUILTIN_MODULES>` Special Word #86

fbartho commented May 16, 2023

IanVS left a comment

IanVS May 17, 2023

fbartho May 17, 2023

IanVS May 17, 2023

fbartho May 17, 2023

IanVS May 17, 2023

Feature: Add <BUILTIN_MODULES> Special Word #86

Feature: Add <BUILTIN_MODULES> Special Word #86

Conversation

fbartho commented May 16, 2023

IanVS left a comment

Choose a reason for hiding this comment

IanVS May 17, 2023

Choose a reason for hiding this comment

fbartho May 17, 2023

Choose a reason for hiding this comment

IanVS May 17, 2023

Choose a reason for hiding this comment

fbartho May 17, 2023

Choose a reason for hiding this comment

IanVS May 17, 2023

Choose a reason for hiding this comment

Feature: Add `<BUILTIN_MODULES>` Special Word #86

Feature: Add `<BUILTIN_MODULES>` Special Word #86